Conversation
giograno
force-pushed
the
dynamodb-kinesis-stream
branch
from
a7cb99b to
35ef51e
Compare
|
I added @baermat as reviewer to give feedback on whether the PR satisfies all the criteria needed for the samples |
thrau
left a comment
thrau
left a comment
There was a problem hiding this comment.
Cool thanks for adding! Just a really minor nit: Could we format the python code with black maybe?
baermat
left a comment
baermat
left a comment
There was a problem hiding this comment.
Looks good, and sorry for the late review, this one slipped through 😬
I have one small change request 🙂
dynamodb-kinesis-stream/Makefile
Outdated
|
|
||
| run: ## Deploy and run the sample locally | ||
| @(test -d .terraform || tflocal init) && tflocal apply --auto-approve | ||
| pip install boto3 |
There was a problem hiding this comment.
The pip install should probably be in the install section. Also it would probably make sense to put this into a venv to not cause problems on the host (we probably should put even the "command" installs into a venv to make it cleaner, but that's how it is right now across the samples). see e.g. https://github.com/localstack/localstack-pro-samples/blob/master/iot-basics/Makefile
3 participants
Adding the sample we pitched today.